Skip to content

Conversation

@Shunpoco
Copy link
Contributor

@Shunpoco Shunpoco commented Sep 27, 2025

Fixes #147088

This PR adds auto py, cpp, and js extra checks into the pre-push script.

  • It checks those non-Rust files only if they are modified in the commit
  • Thanks to auto mode, the pre-push doesn't check them if none of them are modified. It means that it doesn't build venv, nor install node_packages under build/

Note that this PR doesn't add shellcheck and spellcheck, because

  • Currently shellcheck isn't installed by the tidy command unlike venv/node_modules. So it forces developers to take a extra task to enable pre-push hook
  • Spellcheck is built whenever I kick test tidy with the option. If I enables it, developers should wait extra time for running pre-push hook

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 27, 2025
It enables automatic check changes of Python/C++/JS
before pushing the changes to remote repository.
Those checks happen only when the target type of file is changed.
Otherwise it does not install any dependencies (venv and/or node_modules).
Note that shellcheck and spellcheck are not included in this change, because:
1. Unlike venv/node_modules, shellcheck is not installed automatically by the command, and
2. spellcheck is built whenever pre-push script is run, it forces developer to wait extra time
So not to break the current productivity, this commit skips them.
@Shunpoco
Copy link
Contributor Author

r? @Kobzol

@Shunpoco Shunpoco marked this pull request as ready for review September 27, 2025 17:23
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 27, 2025
@Kobzol
Copy link
Member

Kobzol commented Sep 27, 2025

Makes sense. I guess that this also has the possibility of showing bugs in the auto implementation 😆 Thanks!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Sep 27, 2025

📌 Commit aef976e has been approved by Kobzol

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 27, 2025
@Shunpoco
Copy link
Contributor Author

Thanks for the quick review!

this also has the possibility of showing bugs in the auto implementation

Do you mean either of them (or both) are possibly bugs?

Currently shellcheck isn't installed by the tidy command unlike venv/node_modules. So it forces developers to take a extra task to enable pre-push hook

Spellcheck is built whenever I kick test tidy with the option. If I enables it, developers should wait extra time for running pre-push hook

@Kobzol
Copy link
Member

Kobzol commented Sep 27, 2025

Yeah, the auto functionality is new and I don't think that many people use it. But we'll see, so far it was fine.

Zalathar added a commit to Zalathar/rust that referenced this pull request Sep 28, 2025
Add auto extra-checks in pre-push hook

Fixes rust-lang#147088

This PR adds auto py, cpp, and js extra checks into the pre-push script.
- It checks those non-Rust files only if they are modified in the commit
- Thanks to auto mode, the pre-push doesn't check them if none of them are modified. It means that it doesn't build venv, nor install node_packages under build/

Note that this PR doesn't add shellcheck and spellcheck, because
- Currently shellcheck isn't installed by the tidy command unlike venv/node_modules. So it forces developers to take a extra task to enable pre-push hook
- Spellcheck is built whenever I kick test tidy with the option. If I enables it, developers should wait extra time for running pre-push hook
bors added a commit that referenced this pull request Sep 28, 2025
Rollup of 16 pull requests

Successful merges:

 - #142139 (Include additional hashes in src/stage0)
 - #146745 (Clarified error note for usize range matching)
 - #146763 (cg_llvm: Replace some DIBuilder wrappers with LLVM-C API bindings (part 5))
 - #146788 (chore: removes deprecated discord.)
 - #146942 ([rustdoc] Finish getting rid of usages `write_str`)
 - #147002 (rustdoc-search: stringdex update with more packing)
 - #147061 (fix rebasing cycle heads when not reaching a fixpoint)
 - #147066 (Fix tracking issue number for feature(macro_attr))
 - #147081 (doc: fix a typo in platform-support.md)
 - #147082 (formatting_options: fix alternate docs 0b/0o mixup)
 - #147086 (compiletest: Use `PanicHookInfo::payload_as_str` now that it's stable in beta)
 - #147092 (Do not compute optimized MIR if code does not type-check.)
 - #147093 (redox: switch to colon as path separator)
 - #147095 (Library: Remove remaining private `#[repr]` workarounds)
 - #147098 (Add auto extra-checks in pre-push hook)
 - #147110 (Fix typo)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Sep 28, 2025
Rollup of 14 pull requests

Successful merges:

 - #142139 (Include additional hashes in src/stage0)
 - #146745 (Clarified error note for usize range matching)
 - #146763 (cg_llvm: Replace some DIBuilder wrappers with LLVM-C API bindings (part 5))
 - #146788 (chore: removes deprecated discord.)
 - #146942 ([rustdoc] Finish getting rid of usages `write_str`)
 - #147061 (fix rebasing cycle heads when not reaching a fixpoint)
 - #147066 (Fix tracking issue number for feature(macro_attr))
 - #147081 (doc: fix a typo in platform-support.md)
 - #147082 (formatting_options: fix alternate docs 0b/0o mixup)
 - #147086 (compiletest: Use `PanicHookInfo::payload_as_str` now that it's stable in beta)
 - #147093 (redox: switch to colon as path separator)
 - #147095 (Library: Remove remaining private `#[repr]` workarounds)
 - #147098 (Add auto extra-checks in pre-push hook)
 - #147110 (Fix typo)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f31963f into rust-lang:master Sep 28, 2025
10 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Sep 28, 2025
rust-timer added a commit that referenced this pull request Sep 28, 2025
Rollup merge of #147098 - Shunpoco:issue-147088, r=Kobzol

Add auto extra-checks in pre-push hook

Fixes #147088

This PR adds auto py, cpp, and js extra checks into the pre-push script.
- It checks those non-Rust files only if they are modified in the commit
- Thanks to auto mode, the pre-push doesn't check them if none of them are modified. It means that it doesn't build venv, nor install node_packages under build/

Note that this PR doesn't add shellcheck and spellcheck, because
- Currently shellcheck isn't installed by the tidy command unlike venv/node_modules. So it forces developers to take a extra task to enable pre-push hook
- Spellcheck is built whenever I kick test tidy with the option. If I enables it, developers should wait extra time for running pre-push hook
@Shunpoco Shunpoco deleted the issue-147088 branch September 28, 2025 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pre-push hook can check Python code

4 participants